Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #1170, refactor target config objects #1174

Merged
merged 1 commit into from
Mar 5, 2021
Merged

Fix #1170, refactor target config objects #1174

merged 1 commit into from
Mar 5, 2021

Conversation

jphickey
Copy link
Contributor

Describe the contribution
Rework the dynamic content so it is generated entirely via a CMake "configure_file()" command into a C source file (.c) that
can be built as normal.

This removes the need for inline #include statements to pull in data fragments to fill in the data.

Fixes #1170
Fixes #352

Testing performed
Build and run all unit tests, sanity test CFE, confirm versions reported as expected.

Expected behavior changes
This reports multiple event "91" (version info) - one event for each statically linked code module. This now also includes supplemental modules such as "msg" and "sbr" (these were not reported at all before).

This also moves reporting of the "mission config" name from event 91 to event 92 (build info) as it is not really version info. Ideally event 91 should be strictly for source control revision info. Any other environment/config info should be in the other events.

System(s) tested on
Ubuntu 20.04

Additional context
This also attempts to better differentiate/clarify the two different versions that are reported.

The first version is administratively-assigned, traditional "semantic" version (major/minor/rev plus dev build number). This is updated manually and it is what is reported in CFE event 2 during start up as follows:

EVS Port1 66/1/CFE_ES 2: cFS Versions: cfe v6.8.0-rc1+dev348, osal v5.1.0-rc1+dev262, psp v1.5.0-rc158. cFE chksm 65346

The second version is the automatically-obtained "source" version which is gained by introspection of the source code at compile time. This is reported in CFE event 91, as follows:

EVS Port1 66/1/CFE_ES 91: Version Info: Component cfe-core, version git:v6.8.0-rc2-346-g3aa80ed0
EVS Port1 66/1/CFE_ES 91: Version Info: Component osal, version git:v6.0.0-rc1-1-g7576fed6
EVS Port1 66/1/CFE_ES 91: Version Info: Component psp, version git:v1.5.0-rc1-61-gf3dae6c-dirty
EVS Port1 66/1/CFE_ES 91: Version Info: Component msg, version git:v6.8.0-rc2-346-g3aa80ed0
EVS Port1 66/1/CFE_ES 91: Version Info: Component sbr, version git:v6.8.0-rc2-346-g3aa80ed0
EVS Port1 66/1/CFE_ES 91: Version Info: Component resourceid, version git:v6.8.0-rc2-346-g3aa80ed0

Important to note that instead of squeezing this all into a single event 91 as had been done before, this sends a separate event for each component. This means that all components can be done (note this includes msg/sbr/resourceid now, these were not reported before) as well as making it possible to split "cfe-core" into its components as well. It will scale more easily to any number of modular components.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey
Copy link
Contributor Author

Created as a draft because this still needs a few unit test updates to get full coverage on the modifications, but before I do that I wanted to get some other eyes on it to make sure the general concept is good to go.

@jphickey
Copy link
Contributor Author

Also worth a note - there was previous concern as to being potentially too tightly coupled with "git" for the version control info. In this change I have also isolated the version control actions to a separate script (generate_git_module_version.cmake) and also added the capability for a user to provide their own script that does this - so they can call some other tool like SVN or adapt to however they like to do branching/tagging so it provides a useful result. But the default one works fine for the git submodule-based bundle out of the box.

@jphickey jphickey force-pushed the fix-1170-refactor-targetconfig branch from 42b3a69 to 1fed3a1 Compare February 26, 2021 21:10
@jphickey
Copy link
Contributor Author

Rebased to "main" ... but will still hold this one as draft until consensus is reached in nasa/cFS#200 since this will also affected by that decision.

@jphickey
Fix nasa#1170, refactor target config objects
228b914 
Rework the dynamic content so it is generated entirely via a
CMake "configure_file()" command into a C source file (.c) that
can be built as normal.

This removes the need for inline `#include` statements to pull
in data fragments to fill in the data.
@jphickey jphickey force-pushed the fix-1170-refactor-targetconfig branch from 1fed3a1 to 228b914 Compare March 3, 2021 14:54
@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Mar 3, 2021
@jphickey jphickey marked this pull request as ready for review March 3, 2021 14:55
@skliper
Copy link
Contributor

skliper commented Mar 3, 2021

@astrogeco I'm fine with fast-tracking this one if it helps... or review the WIP from Joe as long as we can get the WIP closed out and merged next cycle.

@astrogeco astrogeco added CCB:2021-03-03 and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Mar 3, 2021
@astrogeco
Copy link
Contributor

astrogeco commented Mar 3, 2021 •
edited
Loading

CCB 2021-03-03 APPROVED

  • Using a .c file to avoid developers including a header in different places since these defines are meant to be used in a very specific way.
  • Splits version.cmake into two files generate_build_env.cmake and generate_git_module_version.cmake to accommodate implementations that DO NOT use git
  • Need to better document the customization approach
  • Need to have a separate documentation for the build system (See Document the cmake build system #760)
  • We should start moving away from generating cmake c flags the CORRECT way to do it is to use the target specific files
    • Maybe have example to help people

@skliper skliper added this to the 7.0.0 milestone Mar 5, 2021
@skliper skliper self-assigned this Mar 5, 2021
@skliper skliper mentioned this pull request Mar 5, 2021
Merged
@skliper skliper merged commit b24e9db into nasa:main Mar 5, 2021
@jphickey jphickey deleted the fix-1170-refactor-targetconfig branch March 10, 2021 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper approved these changes

Assignees

@skliper skliper

Labels
None yet
Projects
None yet
Milestone
Caelum
3 participants
@jphickey @skliper @astrogeco